refactor: replace channel with callback of transport in swarm #561

Ma233 · 2024-03-01T04:00:57Z

Please check if the PR fulfills these requirements

Tests for the changes have been added (for bug fixes/features)
Docs have been added / updated (for bug fixes / features)

🔵 What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

🟤 What is the current behavior? (You can also link to an open issue here)

🟢 What is the new behavior (if this is a feature change)?

☢️ Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

ℹ️ Other information

Closes #issue

RyanKung · 2024-03-25T14:02:38Z

plz avoid exposing swarm components such as:

➜  core git:(ma_channel) ✗ pcregrep -rM 'swarm.transport(\n)?.*' src
src/swarm/builder.rs:    /// Sets up the external address for swarm transport.
src/dht/stabilization.rs:        let conns = self.swarm.transport.get_connections();
src/dht/stabilization.rs:                    self.swarm.transport.session_sk(),
src/dht/stabilization.rs:                self.swarm.transport.send_payload(payload).await?;
src/dht/stabilization.rs:                        self.swarm.transport.session_sk(),
src/dht/stabilization.rs:                    self.swarm.transport.send_payload(payload).await?;
src/tests/default/test_connection.rs:    assert!(node1.swarm.transport.get_connection(node2.did()).is_none());
src/tests/default/test_connection.rs:    assert!(node2.swarm.transport.get_connection(node1.did()).is_none());
src/tests/default/mod.rs:            self.swarm.transport.get_connection_ids(),
src/tests/default/mod.rs:            self.swarm.transport.get_connections().len(),
src/tests/default/mod.rs:            assert!(self.swarm.transport.get_connection(addr).is_some());
src/tests/default/test_message_handler.rs:    let connection_1_to_2 = node1.swarm.transport.get_connection(node2.did()).unwrap();
src/tests/default/test_message_handler.rs:    let connection_2_to_3 = node2.swarm.transport.get_connection(node3.did()).unwrap();
src/tests/default/test_message_handler.rs:    let connection_1_to_3 = node1.swarm.transport.get_connection(node3.did());
src/tests/default/test_message_handler.rs:    let connection_1_to_2 = node1.swarm.transport.get_connection(node2.did()).unwrap();
src/tests/default/test_message_handler.rs:    let connection_1_to_2 = node1.swarm.transport.get_connection(node2.did()).unwrap();
src/tests/default/test_message_handler.rs:    let connection_1_to_2 = node1.swarm.transport.get_connection(node2.did()).unwrap();
src/tests/default/test_message_handler.rs:    let connection_1_to_2 = node1.swarm.transport.get_connection(node2.did()).unwrap();
src/message/handlers/connection.rs:        assert!(node1.swarm.transport.get_connection(node3.did()).is_none());
src/message/handlers/connection.rs:        assert!(node1.swarm.transport.get_connection(node3.did()).is_none());
src/message/handlers/connection.rs:            if let Some(t) = node2.swarm.transport.get_connection(node1.did()) {
src/inspect.rs:            let connections = swarm.transport.get_connections();

just hide or wrap the transport, and call functions as

swam.get_connections
swarm.get_connection_ids

RyanKung · 2024-03-25T14:06:15Z

crates/core/src/dht/stabilization.rs


        for (did, conn) in conns.into_iter() {
-            if conn.is_disconnected().await {
+            if matches!(


I dont think match WebrtcConnectionState here is a good idea.

The keyword WebRTC and belonged details is not necessary to be known for swarm / DHT layer.

Just is_connected / is_pending / is_disconnected is fine here

RyanKung · 2024-03-25T14:07:26Z

crates/core/src/dht/stabilization.rs

@@ -88,11 +92,11 @@ impl Stabilization {
                tracing::debug!("STABILIZATION notify_predecessor: {:?}", s);
                let payload = MessagePayload::new_send(
                    msg.clone(),
-                    self.swarm.session_sk(),
+                    self.swarm.transport.session_sk(),


What's the difference between swarm.session_sk and swarm.transport.session_sk ?

RyanKung · 2024-03-25T14:08:00Z

crates/core/src/dht/stabilization.rs

                    s,
                    self.swarm.did(),
                )?;
-                self.swarm.send_payload(payload).await?;
+                self.swarm.transport.send_payload(payload).await?;
            }


Dont expose transport, just wrap it.

RyanKung · 2024-03-25T14:17:17Z

crates/core/src/swarm/callback.rs


-        if payload.transaction.destination == self.did {
+        if payload.transaction.destination == self.transport.dht.did {


transport.dht.did?
swarm.did()?
swarm.dht.did?

RyanKung · 2024-03-25T14:28:20Z

crates/core/src/swarm/mod.rs

-    message_handler: MessageHandler,
-    transport: BoxedTransport<ConnectionOwner, TransportError>,
+    /// Swarm tansport.
+    pub transport: Arc<SwarmTransport>,


the SwarmTransport is actually TransportWithDHT

pub struct SwarmTransport { transport: Transport, session_sk: SessionSk, pub(crate) dht: Arc<PeerRing>, #[allow(dead_code)] measure: Option<MeasureImpl>, }

Maybe just rename SwarmTransport to ConnectionManager is better.

The word Transport Connection, and the way we call them are making confuse here.

Just a metaphor, Transport is a train, Connection is a railway. getting railwayS from a train via the railway station is making no sense.

Key changes: Feature: data channel pool: #562 Feature: Using callback instead MPMC channel: #561 Chore: Make rings-snark optional: #558 Chore: Using forked protocolJs: #556

Ma233 added 5 commits February 29, 2024 11:49

Separate transport operation in swarm

01bfad2

Implement PayloadSender for SwarmTransport

dcf0431

Update handshake api

2f321e6

Update callback mode

820701c

Remove channel

ba3b50a

Ma233 force-pushed the channel branch 5 times, most recently from ad7ec31 to 243601f Compare March 10, 2024 14:25

Use MessageHandler in callback

2bd025e

Ma233 force-pushed the channel branch from 243601f to 2bd025e Compare March 10, 2024 14:28

Ma233 added 2 commits March 11, 2024 14:39

Update rings-core api in rings-node

51149c2

Simplify return type of gen_sorted_ht and gen_pure_dht

70c1036

Ma233 force-pushed the channel branch from 054b431 to 61960b4 Compare March 12, 2024 04:04

Implement a node in default test for monitoring

b800e78

Ma233 force-pushed the channel branch from 61960b4 to b800e78 Compare March 12, 2024 04:05

Ma233 added 3 commits March 12, 2024 20:23

Fix static check of connection handler test

8d751e2

Fix static check of tests

acd8309

Remove not using module and its dependencies

797198e

Ma233 force-pushed the channel branch from 0419cd1 to 797198e Compare March 12, 2024 15:24

Ma233 added 5 commits March 13, 2024 10:27

Fix static check of wasm tests

c0306d2

Fix callback of dummy transport

2d24d43

Should always invoke user defined callbacks

4169caf

Should notify connected after data channel opened

a1efaaa

Fix dummy test datachannel callback behaviour

37e5f81

Ma233 force-pushed the channel branch from 2bf3d26 to 37e5f81 Compare March 19, 2024 08:07

Ma233 added 3 commits March 19, 2024 17:15

Wait for msgs in stabilization and storage tests

121f46b

Fix test_fourth_node_connection by incr wait time

22bb582

Remove meaningless disconnected cleanup test

e6fec62

Ma233 added 5 commits March 19, 2024 22:37

Fix len error in test_extend_data

106e8f3

Fix test_extend_data in storage handler

36edb71

Test case enhance

4ce0ecc

Temeparary skip dummy tests

0c72962

Wait for msg arriving in test

0e02493

Ma233 force-pushed the channel branch from 949b622 to 0e02493 Compare March 22, 2024 03:28

Fix handshake_on_both_sides test

a12bd72

Ma233 force-pushed the channel branch 2 times, most recently from 77aa542 to d3958f7 Compare March 25, 2024 03:28

Ma233 changed the title ~~[WIP] refactor: replace channel with callback of transport in swarm~~ refactor: replace channel with callback of transport in swarm Mar 25, 2024

Fix receiver dropping bug in test_stabilization_final_dht

7dee3b0

Ma233 force-pushed the channel branch from d3958f7 to 7dee3b0 Compare March 25, 2024 03:34

RyanKung reviewed Mar 25, 2024

View reviewed changes

Upgrade stabilization api

cb9e035

Ma233 force-pushed the channel branch from 604bd7d to 08d86bb Compare March 28, 2024 01:59

Hide the SwarmTransport in the Swarm of rings-core

5b29928

Ma233 force-pushed the channel branch from 08d86bb to 5b29928 Compare March 28, 2024 02:16

RyanKung approved these changes Mar 28, 2024

View reviewed changes

Ma233 merged commit 3ffcdd2 into RingsNetwork:master Mar 28, 2024
5 checks passed

Ma233 deleted the channel branch March 28, 2024 03:54

RyanKung mentioned this pull request Mar 28, 2024

chore: bump version to 0.6.0 #563

Merged

RyanKung added a commit that referenced this pull request Mar 28, 2024

bump version to 0.6.0 (#563)

034a47e

Key changes: Feature: data channel pool: #562 Feature: Using callback instead MPMC channel: #561 Chore: Make rings-snark optional: #558 Chore: Using forked protocolJs: #556

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: replace channel with callback of transport in swarm #561

refactor: replace channel with callback of transport in swarm #561

Ma233 commented Mar 1, 2024

RyanKung commented Mar 25, 2024 •

edited

Loading

RyanKung Mar 25, 2024

RyanKung Mar 25, 2024

RyanKung Mar 25, 2024

RyanKung Mar 25, 2024

RyanKung Mar 25, 2024 •

edited

Loading

RyanKung Mar 25, 2024 •

edited

Loading


		if payload.transaction.destination == self.did {
		if payload.transaction.destination == self.transport.dht.did {

refactor: replace channel with callback of transport in swarm #561

refactor: replace channel with callback of transport in swarm #561

Conversation

Ma233 commented Mar 1, 2024

Please check if the PR fulfills these requirements

🔵 What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

🟤 What is the current behavior? (You can also link to an open issue here)

🟢 What is the new behavior (if this is a feature change)?

☢️ Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

ℹ️ Other information

RyanKung commented Mar 25, 2024 • edited Loading

RyanKung Mar 25, 2024

Choose a reason for hiding this comment

RyanKung Mar 25, 2024

Choose a reason for hiding this comment

RyanKung Mar 25, 2024

Choose a reason for hiding this comment

RyanKung Mar 25, 2024

Choose a reason for hiding this comment

RyanKung Mar 25, 2024 • edited Loading

Choose a reason for hiding this comment

RyanKung Mar 25, 2024 • edited Loading

Choose a reason for hiding this comment

RyanKung commented Mar 25, 2024 •

edited

Loading

RyanKung Mar 25, 2024 •

edited

Loading

RyanKung Mar 25, 2024 •

edited

Loading